fix: Use aria-disabled instead of disabled for undo/redo #11379
Conversation
There was a problem hiding this comment.
It works! 👍
I'm wondering, should this logic exist in Button and/or IconButton? Does this bug also exist in other places where we're using buttons with the disabled prop? It would be nice if developers could set the disabled prop on our Button component and rely on it doing the Right Thing™.
noisysocks
added
the
[Focus] Accessibility (a11y)
|
It probably should, but I tried that in the past with some other buttons (in the Datepicker's |
| redo: () => dispatch( 'core/editor' ).redo(), | ||
| withDispatch( ( dispatch, ownProps ) => ( { | ||
| redo: () => { | ||
| // If there are no redo levels this is a no-op, because we don't actually |
There was a problem hiding this comment.
In treating EditorHistoryRedo as the presentational component of the Presentational / Container pair, it seems like if it's meant to be a disabled behavior, it should be considered by the component in deciding to invoke the onClick callback.
Even something like:
onClick={ hasRedo ? redo : undefined }It's not so much a concern here in that the components are so tied to the specific editor use case of Undo/Redo, but is a good rule of thumb in delegating where logic should occur between the underlying component and its enhancer(s).
There was a problem hiding this comment.
Oh, good point. Follow-up PR with a fix is here: #11428.
* chore: Improve undo/redo no-op See: https://github.com/WordPress/gutenberg/pull/11379\#discussion_r230406108 * Do not wrap dispatch undo/redo
Fixes #3486
Description
Change the undo/redo buttons from using the
disabledattribute and instead use thearia-disabledattribute; this prevents focus loss when the undo/redo actions are unavailable.Note that there are no visual/functional differences here aside from screenreaders now announcing that the button becomes unavailable once disabled and focus is not lost as before.